Skip to content

NH-3489 improve GetEffectiveParameterLocations performance #214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 22, 2015
Merged

NH-3489 improve GetEffectiveParameterLocations performance #214

merged 1 commit into from
Feb 22, 2015

Conversation

MrJul
Copy link
Contributor

@MrJul MrJul commented Jun 28, 2013

Added a cache of backtrace-to-location with the BackTrackCacheParameterList class, that is used by SqlCommandImpl. GetEffectiveParameterLocations still use the old code if a standard collection is passed for compatibility reasons.

The only public change is SqlCommandImpl.SqlQueryParametersList that now has a return type of IList rather than List.

A performance test case has been added. A ~40% performance improvement has been measured for the test scenario.

@hazzik
Copy link
Member

hazzik commented Nov 16, 2014

Thanks for the contribution, @MrJul. But I would like to have more explicit solution

@hazzik hazzik closed this Nov 16, 2014
@MrJul
Copy link
Contributor Author

MrJul commented Nov 16, 2014

@hazzik what to do mean by "more explicit"? After all, I'm explicitly caching :)

I'm really interested in getting this solved, this affects so many queries in one of our application that I've been maintaining a custom build for more than a year.

@hazzik
Copy link
Member

hazzik commented Nov 16, 2014

I mean that I do not like the fact that you hide your magic cache behind
IList interface.

@MrJul
Copy link
Contributor Author

MrJul commented Nov 16, 2014

It was to keep maximum backward compatibility for callers using it as a list. There's no observable side effect for them, in fact it could even become an option if necessary (use a plain List or the Cache list behind the IList).

@hazzik
Copy link
Member

hazzik commented Nov 16, 2014

Ok, @MrJul. Can you please rebase on top of master branch?

@hazzik hazzik reopened this Nov 16, 2014
@hazzik hazzik added this to the 4.1.0 milestone Nov 18, 2014
@MrJul
Copy link
Contributor Author

MrJul commented Nov 18, 2014

Sorry for the delay, that's done!

@hazzik
Copy link
Member

hazzik commented Nov 18, 2014

@MrJul SharpTestEx has been removed on master, can you please update tests?

@MrJul
Copy link
Contributor Author

MrJul commented Nov 18, 2014

@hazzik I don't get it, I don't use SharpTestEx in my tests for NH-3489. The compilation seems to already fail in the current NH master at https://github.com/nhibernate/nhibernate-core/blob/master/src/NHibernate.Test/NHSpecificTest/NH3570/UniFixture.cs

@hazzik
Copy link
Member

hazzik commented Nov 18, 2014

Ok, sorry then

@hazzik
Copy link
Member

hazzik commented Nov 18, 2014

@MrJul sorry for asking, but could you please rebase on top of master again?

@MrJul
Copy link
Contributor Author

MrJul commented Nov 18, 2014

No problem @hazzik that's done!

@hazzik hazzik merged commit 8fc922d into nhibernate:master Feb 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants